Add precipitation histogram to prognostic run report #1271
Conversation
| @@ -229,16 +227,6 @@ def _assign_diagnostic_time_attrs( | |||
| return diagnostics_ds | |||
|
|
|||
|
|
|||
| def dump_nc(ds: xr.Dataset, f): | |||
There was a problem hiding this comment.
We switched to using the vcm.dump_nc version of this a while ago, so this func is unused.
| logger.info("Computing histograms for physics diagnostics") | ||
| counts = xr.Dataset() | ||
| for varname in prognostic.data_vars: | ||
| count, bins = np.histogram( |
There was a problem hiding this comment.
I found using np.histogram much faster (and simpler code-wise) than doing an xarray groupby.
| def plot_1d( | ||
| run_diags: RunDiagnostics, varfilter: str, run_attr_name: str = "run", | ||
| ) -> HVPlot: | ||
| def plot_1d(run_diags: RunDiagnostics, varfilter: str) -> HVPlot: |
There was a problem hiding this comment.
deleted unused argument run_attr_name
There was a problem hiding this comment.
Looks great. Apart from some minor comments below, I had some thoughts
- I was pretty confused about why the metric functions returned
xr.Datasetrather than floats, before realizing that theto_dictfunction looks at the units and values. It would be clearer if the metrics functions returned dataclasses with.valueand.unitsattrs, so that we are passing the minimal amount of data around. - It would be great to enhance the visibility of the scalar metrics, so that they are amongst the first things we see. e.g. by showing a table instead of the histograms. The histograms are kind of useless and ugly IMO, especially for a single run report.
workflows/prognostic_run_diags/fv3net/diagnostics/prognostic_run/compute.py
Show resolved
Hide resolved
workflows/prognostic_run_diags/fv3net/diagnostics/prognostic_run/metrics.py
Outdated
Show resolved
Hide resolved
workflows/prognostic_run_diags/fv3net/diagnostics/prognostic_run/metrics.py
Show resolved
Hide resolved
yeah I agree the
ouch, poor histogram ;) It would be helpful to have the verification data on there, I agree. The example is particularly noisy since it's a short run. But yeah, a table showing the percentiles for each run would be quite useful. |
sorry. I didn't mean your histograms, i meant the bar charts with the metrics on the main page. The histograms are great! |
haha sounds good. I agree the bar charts are hard to parse. I'll work on adding a metrics table with the things we actually care about. |
Useful to show a histogram of precipitation on prognostic run reports. Example report HERE.
Significant internal changes:
process_diagnostics.htmlpage to the prog run report. Moved diurnal cycle there, and added plot of precipitation histogram.Sped up report test by only creating report for a single run (since no longer require >1 run for report generation).